Closed Bug 1371511 Opened 8 years ago Closed 8 years ago

InvalidArrayIndex_CRASH {@ mozilla::a11y::NotificationController::WillRefresh]

Categories

(Core :: Disability Access APIs, defect)

defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox-esr52 --- unaffected
firefox54 --- wontfix
firefox55 --- fixed
firefox56 --- fixed

People

(Reporter: tsmith, Assigned: eeejay)

References

(Blocks 1 open bug)

Details

(4 keywords, Whiteboard: [fuzzblocker][adv-main55+][post-critsmash-triage])

Attachments

(1 file)

Attached file test_case.html
This was found on Ubuntu 16.04 with m-c 20170606181015 6cd0639e02 I assume this is a safe crash but I'm not 100% so I've marked it s-s. This test case requires the fuzzPriv extension to reproduce. ==23810==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x00000051b5b6 bp 0x7ffee27087f0 sp 0x7ffee2708680 T0) #0 0x51b5b5 in MOZ_CrashPrintf src/mfbt/Assertions.cpp:63:3 #1 0x7f506af3b02f in InvalidArrayIndex_CRASH(unsigned long, unsigned long) src/xpcom/ds/nsTArray.cpp:26:3 #2 0x7f50741205d6 in ElementAt src/obj-firefox/dist/include/nsTArray.h:1048:7 #3 0x7f50741205d6 in operator[] src/obj-firefox/dist/include/nsTArray.h:1086 #4 0x7f50741205d6 in mozilla::a11y::NotificationController::WillRefresh(mozilla::TimeStamp) src/accessible/base/NotificationController.cpp:738 #5 0x7f50718f0045 in nsRefreshDriver::Tick(long, mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:1791:12 #6 0x7f50718fea85 in mozilla::RefreshDriverTimer::TickRefreshDrivers(long, mozilla::TimeStamp, nsTArray<RefPtr<nsRefreshDriver> >&) src/layout/base/nsRefreshDriver.cpp:301:7 #7 0x7f50718fe742 in mozilla::RefreshDriverTimer::Tick(long, mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:322:5 #8 0x7f5071900e3b in RunRefreshDrivers src/layout/base/nsRefreshDriver.cpp:754:5 #9 0x7f5071900e3b in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver(mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:667 #10 0x7f50718fc147 in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::ParentProcessVsyncNotifier::Run() src/layout/base/nsRefreshDriver.cpp:513:20 #11 0x7f506b01141e in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1321:14 #12 0x7f506b01d858 in NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:472:10 #13 0x7f506bde9c91 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:96:21 #14 0x7f506bd46d90 in RunInternal src/ipc/chromium/src/base/message_loop.cc:238:10 #15 0x7f506bd46d90 in RunHandler src/ipc/chromium/src/base/message_loop.cc:231 #16 0x7f506bd46d90 in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:211 #17 0x7f507125d68f in nsBaseAppShell::Run() src/widget/nsBaseAppShell.cpp:156:27 #18 0x7f507491dce1 in nsAppStartup::Run() src/toolkit/components/startup/nsAppStartup.cpp:283:30 #19 0x7f5074aee334 in XREMain::XRE_mainRun() src/toolkit/xre/nsAppRunner.cpp:4569:22 #20 0x7f5074aefea0 in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) src/toolkit/xre/nsAppRunner.cpp:4749:8 #21 0x7f5074af11f1 in XRE_main(int, char**, mozilla::BootstrapConfig const&) src/toolkit/xre/nsAppRunner.cpp:4844:21 #22 0x4eb5a3 in do_main src/browser/app/nsBrowserApp.cpp:236:22 #23 0x4eb5a3 in main src/browser/app/nsBrowserApp.cpp:309 #24 0x7f50869a582f in __libc_start_main /build/glibc-9tT8Do/glibc-2.23/csu/../csu/libc-start.c:291 #25 0x41d0f8 in _start (m-c-1496772615-asan-opt/firefox+0x41d0f8)
is there any guidance I can follow to install fuzzPriv extension on my local build?
The plugin xpi can be found here: https://www.squarefree.com/extensions/domFuzzLite3.xpi You can also do it by copying the contents of this extension directory[1] in to <firefox_profile_dir>/extensions/domfuzz@squarefree.com/ You can also use ffpuppet[2] to simplify launching the browser with a fresh temporary profile and cleaning up after. It can install the extension automatically. I'd recommend using ffpuppet because it prevents breaking your existing profile when using custom prefs.js and extensions. I use ffpuppet and so does our automation. [1] https://github.com/MozillaSecurity/domfuzz/tree/master/dom/extension [2] https://github.com/MozillaSecurity/ffpuppet
This is a safe crash, but I'm a little concerned that this code crashes so easily.
here's a stack of the error: 1) handling child document fails to bind and gets shutdown [1] 2) then MaybeShutdownAccService shutdowns the accessibility service [2], I assume because GC collected all stuff and there's no reason to keep a11y running 3) we've got shutdown on the run -> crash An easy fix would be to add a mDocument null check after each child document shutdown. On the other hand I'm not 100% sure there's nothing bogus here. So that if that was the garbage collector who's cleared all stuff, then I would expect the total shutdown happens having GC stuff on the stack, but we have WillRefresh which is in a good state (mDocument is not null), which indicates that a11y is still running. [1] https://dxr.mozilla.org/mozilla-central/source/accessible/base/NotificationController.cpp#763 [2] https://dxr.mozilla.org/mozilla-central/source/accessible/base/nsAccessibilityService.cpp?q=MaybeShutdownAccService&redirect_type=direct#1826
Ok, here's no-GC-on-stack issue explanation. GC kills XPCOM's accessibility service, it schedules a11y shutdown in 100ms [1], and then WillRefresh triggers right before the scheduled shutdown. Not yet clear, why child document fails to bind, since technically nothing shutdowns accessibility yet. [1] https://dxr.mozilla.org/mozilla-central/source/accessible/xpcom/xpcAccessibilityService.cpp?q=mShutdownTimer-%3EInitWithFuncCallback%28ShutdownCallback%2C+this%2C+100%2C&redirect_type=single#82
cc'ing Eithan for ideas.
I think my fix in bug 1330765 may remedy this too..
This appears to be fixed, I haven't seen it since the patch in bug 1330765 landed. How should we close this one? as a dup?
Flags: needinfo?(eitan)
Yup, makes sense. If this pops up again, open a new bug.
Status: NEW → RESOLVED
Closed: 8 years ago
Flags: needinfo?(eitan)
Resolution: --- → DUPLICATE
sorry for being nitpicky here, but it doesn't seem a dupe, at least because they have different stack traces. I'm good to call it fixed if you cannot reproduce it any longer, however it's not that obvious with me how bug 1330765 helps it.
Resolution: DUPLICATE → FIXED
It would be good to get some consensus here just to make sure we're not missing anything. Can you please expand on why bug 1330765 may have solved this?
Flags: needinfo?(eitan)
Group: dom-core-security → core-security-release
(In reply to Ryan VanderMeulen [:RyanVM] from comment #11) > It would be good to get some consensus here just to make sure we're not > missing anything. Can you please expand on why bug 1330765 may have solved > this? The case seems to be similar: sub-documents loaded while starting and stopping a11y. Also, I am only able to reproduce this crash when I revert the patch from bug 1330765.
Flags: needinfo?(eitan)
Assignee: nobody → eitan
Depends on: 1330765
Target Milestone: --- → mozilla56
Whiteboard: [fuzzblocker] → [fuzzblocker][adv-main55+]
Flags: qe-verify-
Whiteboard: [fuzzblocker][adv-main55+] → [fuzzblocker][adv-main55+][post-critsmash-triage]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: